Fix: Add automatic image limit handling for Claude models (#8800) #8801
Conversation
- Add maxImages property to ModelInfo type and Anthropic models - Implement automatic image trimming when limit is exceeded - Add warnings when images are trimmed from conversation - Add comprehensive tests for image limit handling Fixes #8800
dosubot
bot
added
size:XL
Review SummaryI've reviewed the PR and found 2 issues that should be addressed: Issues to Fix
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| } | ||
| } | ||
|
|
||
| const warningMessage = `⚠️ Removed ${trimmedCount} image(s) from conversation history to stay within the model's limit of ${maxImages} images. The oldest images were removed first.` |
There was a problem hiding this comment.
The warning message is hardcoded in English. Consider using the translation function (e.g. t('...')) for this user‐facing string to support internationalization.
| const warningMessage = `⚠️ Removed ${trimmedCount} image(s) from conversation history to stay within the model's limit of ${maxImages} images. The oldest images were removed first.` | |
| const warningMessage = t(`⚠️ Removed ${trimmedCount} image(s) from conversation history to stay within the model's limit of ${maxImages} images. The oldest images were removed first.`) |
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
hannesrudolph
added
the
Issue/PR - Triage
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| // Check if we need to trim images after adding the new message | ||
| const modelInfo = this.api.getModel().info | ||
| if (modelInfo.supportsImages && modelInfo.maxImages) { | ||
| const imageCount = countImagesInConversation(this.apiConversationHistory) | ||
| if (imageCount > modelInfo.maxImages) { | ||
| const trimResult = trimImagesFromConversation(this.apiConversationHistory, modelInfo) | ||
|
|
||
| // Update conversation history with trimmed messages | ||
| this.apiConversationHistory = trimResult.messages | ||
|
|
||
| // Notify user about trimmed images | ||
| if (trimResult.warningMessage) { | ||
| await this.say("text", trimResult.warningMessage, undefined, false, undefined, undefined, { | ||
| isNonInteractive: true, | ||
| }) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This image trimming logic is duplicated at two locations (here in addToApiConversationHistory at lines 606-623, and again in attemptApiRequest at lines 2593-2610). This duplication creates multiple issues: (1) Users may see duplicate warning messages when images are trimmed since both locations call say() with the warning; (2) Code maintenance becomes more complex as any fixes or improvements need to be applied in two places; (3) The implementations could drift apart over time. Consider consolidating this logic into a single helper method that can be called from both locations, or better yet, perform the trimming only once before the API request is made in attemptApiRequest.
| /** | ||
| * Check if adding new content would exceed the image limit | ||
| */ | ||
| export function wouldExceedImageLimit( | ||
| currentMessages: ApiMessage[], | ||
| newContent: Anthropic.Messages.ContentBlockParam[], | ||
| modelInfo: ModelInfo, | ||
| ): boolean { | ||
| if (!modelInfo.supportsImages || !modelInfo.maxImages) { | ||
| return false | ||
| } | ||
|
|
||
| const currentImageCount = countImagesInConversation(currentMessages) | ||
| let newImageCount = 0 | ||
|
|
||
| if (Array.isArray(newContent)) { | ||
| for (const block of newContent) { | ||
| if (block.type === "image") { | ||
| newImageCount++ | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return currentImageCount + newImageCount > modelInfo.maxImages | ||
| } |
There was a problem hiding this comment.
The wouldExceedImageLimit function is never called anywhere in the codebase. This appears to be dead code that was likely intended for proactive checking before adding content, but the implementation uses direct trimming instead. Consider either removing this unused function or utilizing it before adding messages to provide more granular control over when trimming occurs.
2 participants
This PR attempts to address Issue #8800. Feedback and guidance are welcome.
Problem
Claude 4.x models have a hard limit of 20 images maximum in the context window. When users run browser-based test campaigns with more than 20 screenshots, they encounter a fatal error:
too many images and documents: 21 + 0 > 20Solution
Implemented automatic image limit handling that:
maxImagesproperty to model configurations (set to 20 for all Claude/Anthropic models)Changes
maxImagesproperty toModelInfotype and set it for all Anthropic modelsimage-limit-handler.tsmodule with functions to count and trim imagesTask.tsto check and enforce image limits at two critical points:Testing
Fixes #8800
Important
Adds automatic image limit handling for Claude models, trimming excess images and notifying users, with new tests and integration in task workflow.
maxImagesproperty toModelInfofor Anthropic models, set to 20.Task.tswhen image count exceedsmaxImages.image-limit-handler.tsmodule withcountImagesInConversation(),trimImagesFromConversation(), andwouldExceedImageLimit()functions.image-limit-handler.spec.tsfor image limit handling.Task.tsto enforce image limits when adding messages and before API requests.This description was created by
for 0a99134. You can customize this summary. It will automatically update as commits are pushed.